Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Staking limits in Bitcoin Depositor contract #253

Merged
merged 19 commits into from
Mar 6, 2024
Merged

Conversation

nkuba
Copy link
Member

@nkuba nkuba commented Feb 16, 2024

This PR enhances #91.
Depends on keep-network/tbtc-v2#787
Depends on keep-network/tbtc-v2#791

Introduction

In this PR we introduce a mechanism for the dApp to throttle stake request initialization.

The staking flow for Bitcoin is asynchronous, consisting of a couple of stages between the user entering the staking form in the dApp and the tBTC being deposited in the stBTC vault.

Since the stBTC vault introduced a limit for maximum total assets under management, there is a risk that the user will initialize staking that won't be able to finalize in stBTC, due to concurrent other users' stakes. We need to reduce such risk by throttling stake initialization flow in the dApp.

Soft Cap and Hard Cap Limits

Hard Cap

stBTC contract defines a maximum total assets limit, that cannot be exceeded with new tBTC deposits. This is considered a hard limit, that when reached no new deposits can be made, and stake requests in the Bitcoin Depositor contract will be queued. These queued requests will have to wait until the limit is raised or other users withdraw their funds, making room for new users.

Soft Cap

Bitcoin Depositor Contract defines a maximum total assets soft limit, which is assumed to be much lower than the hard cap. The limit is used to throttle stakes and use a difference between hard cap and soft cap as a buffer for possible async stakes coming from the dApp.

Stake Amount Limits

Minimum Stake Limit

The Bitcoin Depositor contract defines a minimum stake limit, that is used to define a minimum amount of a stake. The limit has to be higher than the deposit dust threshold defined in the tBTC Bridge, which is validated on the deposit reveal.
The minimum stake limit has to take into account the minimum deposit limit defined in the stBTC contract, and consider that the amount of tBTC deposited in the stBTC vault on stake finalization will be reduced by tBTC Bridge fees and Depositor fee.

Maximum Stake Limit

The Bitcoin Depositor contract defines a maximum stake limit, which is the maximum amount of a single stake request. This limit is used to introduce granularity to the stake amount and reduce the possibility of a big stake request using the whole soft limit shared across other concurrent stake requests.

Usage in dApp

The limits should be validated in the dApp staking flow to reduce the possibility of user stakes being stuck in the queue.

The contract exposes two functions to be used in the dApp minStakeInSatoshi and maxStakeInSatoshi, for convenience the result is returned in the satoshi precision.

Flow

image
source: Figjam

The staking flow in the dApp is asynchronous and there is a short period
of time between a deposit funding transaction is made on Bitcoin chain
and revealed to this contract.This limit is used to gain better control
on the stakes queue, and reduce a risk of concurrent stake requests
made in the dApp being blocked by another big deposit.
stBTC contract introduces limits for total deposits amount. Due to
asynchronous manner of the staking flow, this contract needs to track
balance of queued stake requests to ensure new stake request are
not initialized if they won't be able to finalize.
stBTC contract defines a maximum total assets limit held by the protocol
that new deposits cannot exceed (hard cap). Due to the asynchronous
manner of Bitcoin deposits process we introduce a soft limit (soft cap)
set to a value lower than the hard cap to let the dApp initialize
Bitcoin deposits only up to the soft cap limit.
The function calculates maximum amount for a stake user should be able
to make in the dApp.
It takes into consideration the maximum total assets soft limit and
queued requests balance.
Define a minimum value for a stake, to use in dApp.
The value has to be greater than the deposit dust threshold set in the
tBTC Bridge, to be able to reveal the deposit.
maxSingleStakeAmount and maxTotalAssetsSoftLimit are defined with
uint256 type, we need to make sure the update functions operate on the
same type, so the values can be updated.
@nkuba nkuba changed the base branch from main to tbtc-depositor February 16, 2024 12:18
Read variables to memory from storage once to optimize gas cost usage.

For queueStake gas usage was reduced from `142 767` to `113 595`
For cancelQueuedStake gas usage was reduced from `66 174` to `66 013`
To avoid confusion of mixing precisions in the contract, we decidwed to
use tBTC precision consistently across the contract and its' API.
It will be dApp/SDK responsibility to convert to satoshi precision in
staking flow.
nkuba added a commit to keep-network/tbtc-v2 that referenced this pull request Feb 27, 2024
The amount may be useful for integrators for validations of the deposit
amount. See example use case in thesis/acre#253 (comment)

It turns out that this version where we read from the storage costs
`117 366` gas. Which is less than an alternative approach:
```
        initialDepositAmount =
            fundingTx
                .outputVector
                .extractOutputAtIndex(reveal.fundingOutputIndex)
                .extractValue() *
            SATOSHI_MULTIPLIER;
```
which costs `117 601` gas.
The abstract contract exposes the values, so the implementation doesn't
have to interact with the bridge and do any precision conversion.
These changes depend on:
keep-network/tbtc-v2#791
@nkuba nkuba requested a review from lukasz-zimnoch February 27, 2024 12:10
@nkuba nkuba self-assigned this Feb 28, 2024
lukasz-zimnoch added a commit to keep-network/tbtc-v2 that referenced this pull request Feb 28, 2024
In this PR we introduce two changes to the AbstractDepositorContract
around the deposit amounts.

### Expose minimum deposit amount
Integrators may need a minimum deposit amount to introduce validation if
the amount the user wants to deposit meets the Bridge requirement of a
deposit to exceed the dust threshold.
Originally proposed in
thesis/acre#253 (comment).

### Return initial deposit amount from _initializeDeposit function

The amount may be useful for integrators for validations of the deposit
amount. Originally proposed in
thesis/acre#253 (comment).
    
It turns out that this version where we read from the storage:
```solidity
        initialDepositAmount =
            bridge.deposits(depositKey).amount *
            SATOSHI_MULTIPLIER;
```
 costs `117 366` gas.
Which is less than an alternative approach:
```solidity
            initialDepositAmount =
                fundingTx
                    .outputVector
                    .extractOutputAtIndex(reveal.fundingOutputIndex)
                    .extractValue() *
                SATOSHI_MULTIPLIER;
```
which costs `117 601` gas.

We return `initialDepositAmount` from two functions:
`_initializeDeposit` and `_finalizeDeposit`. I tried to introduce a
getter function:
```solidity
    function _getInitialDepositAmount(
        uint256 depositKey
    ) internal view returns (uint256) {
        IBridgeTypes.DepositRequest memory deposit = bridge.deposits(
            depositKey
        );
        require(deposit.revealedAt != 0, "Deposit not initialized");

        return deposit.amount * SATOSHI_MULTIPLIER;
    }
```
and removed `initialDepositAmount` return from `_initializeDeposit` and
`_finalizeDeposit` functions.
Unfortunately, the overall cost in the reference BitcoinDepositor
implementation from thesis/acre#253 wasn't
improved:
BEFORE:
```
+ initializeStake --> 143 004
+ finalizeStake --> 195 178
= total --> 338 182
```
AFTER:
```
+ initializeStake --> 142 912
+ finalizeStake --> 197 960
= total --> 340 872
```
The newest version contains changes from keep-network/tbtc-v2#791
@dimpar
Copy link
Member

dimpar commented Mar 4, 2024

Two high level questions:

  1. What happens if someone would deposit outside of the dApp and bypass the soft limit in dApp?
  2. Have you considered coupling soft limit with the maximum total assets percentage wise? E.g. soft limit = 60% * stBTC.maximumTotalAssets()? This percentage value can be managed by the owner. I think this way there would be less manual labor and tracking of the soft limit, because in cases when the maximumTotalAssets changes, most likely the soft limit will also need to be changed. Setting a soft limit as % would trigger the change automatically.

@nkuba
Copy link
Member Author

nkuba commented Mar 6, 2024

1. What happens if someone would deposit outside of the dApp and bypass the soft limit in dApp?

They may reach the hard limit and end up in the queue. There is a possibility of such action, but we don't consider it as likely to happen, as depositing outside of the dApp is complicated.

2. Have you considered coupling soft limit with the maximum total assets percentage wise? E.g. `soft limit = 60% * stBTC.maximumTotalAssets()`? This percentage value can be managed by the owner. I think this way there would be less manual labor and tracking of the soft limit, because in cases when the `maximumTotalAssets` changes, most likely the soft limit will also need to be changed. Setting a soft limit as % would trigger the change automatically.

I've been considering that but ended up with the assumption that Governance will need to consider them both anyway when adjusting system parameters and the current approach gives the most flexibility to define them independently without introducing additional logic of multiplications.

dimpar
dimpar previously approved these changes Mar 6, 2024
Base automatically changed from tbtc-depositor to main March 6, 2024 11:25
@dimpar dimpar dismissed their stale review March 6, 2024 11:25

The base branch was changed.

@nkuba nkuba marked this pull request as ready for review March 6, 2024 13:02
Copy link

netlify bot commented Mar 6, 2024

Deploy Preview for acre-dapp-testnet ready!

Name Link
🔨 Latest commit ff3af34
🔍 Latest deploy log https://app.netlify.com/sites/acre-dapp-testnet/deploys/65e86951288f610008211b2d
😎 Deploy Preview https://deploy-preview-253--acre-dapp-testnet.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@dimpar dimpar merged commit c93969e into main Mar 6, 2024
20 checks passed
@dimpar dimpar deleted the tbtc-depositor-limits-2 branch March 6, 2024 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants